Ship default resource requests/limits for cron and consumers - #47

Medium: This 1Gi memory limit is the actual behavior-risk half of the deviation from legacy BestEffort. The requests are harmless (scheduling hints), but a limit converts "could burst" into "gets OOMKilled": Shopsys cron jobs include memory-heavy work (feed generation, image processing, elasticsearch export), and PHP CLI commonly runs with memory_limit=-1, so nothing inside the container will stop a job before the kernel does. A cron that today peaks at, say, 1.2Gi on a pilot project will silently start dying after upgrading to this chart version.

The PR description already flags the open "verify against production metrics" task — good. Two suggestions:

• Treat that verification as a gate before a release that real projects consume, since the failure mode (nightly cron OOMKilled) is quiet and delayed.
• Consider adding one sentence to deviation 21 telling upgraders to check their crons' peak memory usage before adopting the defaults.

Not blocking — the value is overridable per environment and the trade-off is a judgment call.

— Claude Agent (CR)

Agreed that the 1Gi limit is the risky half. Two-part response in d74fdc4:

• Deviation entry 21 now spells out the upgrade check: QoS moves from BestEffort to Burstable, bursts past 1Gi are OOMKilled instead of merely evictable, and upgraders should check their crons'/consumers' peak memory before relying on the defaults (with resources: null documented as the escape hatch back to legacy behavior).
• Whether 1Gi actually covers real production peaks (feed generation, image processing, ES export) cannot be verified from this repo — that needs production metrics. Escalating the "verify against metrics before tagging a release" gate to the maintainer as an open question rather than guessing.

Low (verified by rendering): the per-instance override path has an asymmetric null footgun. Setting resources: null on a consumer instance cleanly opts that instance out (the with $consumer.resources guard drops the block → BestEffort), but a nested null like:

consumers:
  instances:
    - name: heavy
      transports: t
      resources:
        limits:
          memory: null

survives mergeOverwrite (Helm's null-stripping coalesce doesn't reach inside list items) and renders a literal memory: null, which fails K8s schema validation (kubeconform 1.31.0: Deployment consumer-... is invalid ... resources/limits/memory). The chart-level cron.resources path is fine — Helm's coalesce strips the null there (renders limits: {}, verified).

Nothing to fix in the template; just consider documenting in docs/values.md that removing limits for a single consumer instance requires nulling/replacing the whole resources map, not a nested key.

— Claude Agent (CR)

Reproduced the footgun by rendering: per-instance resources: null cleanly drops the block, while a nested resources: {limits: {memory: null}} survives mergeOverwrite and renders a literal memory: null. Documented in docs/values.md (d74fdc4): to remove limits for a single consumer instance, null or replace the whole resources map, never a nested key. No template change — the chart-level path is unaffected (Helm's coalesce strips nulls there), and guarding against nested nulls in the template would add complexity for a case the docs now cover.

Low: the entry records the numbers but not the operational implication of the deviation: pods move from BestEffort to Burstable QoS, and with the memory request (300Mi) far below the limit (1Gi) nodes can overcommit — under memory pressure these pods remain early eviction candidates whenever usage exceeds the request, and anything bursting past 1Gi is now OOMKilled instead of merely evictable. One added sentence ("QoS changes from BestEffort to Burstable; check your crons' peak memory before relying on the 1Gi default") would make the upgrade checklist complete, since this file is exactly where upgraders look for behavior changes.

— Claude Agent (CR)

Done in d74fdc4 — deviation entry 21 now states the QoS change (BestEffort → Burstable), the overcommit implication of request 300Mi << limit 1Gi, that bursts past 1Gi are OOMKilled instead of merely evictable, the advice to check peak memory before relying on the defaults, and the resources: null opt-out.

Nit: the default and deep-merge cases are covered, but the opt-out escape hatch is not — a test setting cron.resources: null and asserting the container has no resources key (and/or the equivalent whole-map resources: null on a consumer instance in component_schema_test.yaml) would pin the behavior projects will rely on to restore legacy BestEffort. I verified by rendering that both currently work; a test keeps them working.

— Claude Agent (CR)

Added in d74fdc4: cron_test.yaml pins cron.resources: null → no resources key on the container, and component_schema_test.yaml pins the per-instance equivalent (resources: null on a consumer instance). Both pass (64/64); golden snapshots untouched since no rendered output changed.